Necessary changes required to be able to instantiate a TrialFESpace with an instance of DistributedCellField #185
Conversation
with an instance of DistributedCellField
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for creating TrialFESpace with DistributedCellField objects as boundary/initial data. The key changes include:
- Added two new
TrialFESpaceconstructor overloads that acceptDistributedCellFieldarguments - Added a new
interpolate_dirichlet!method to handleDistributedCellFieldinterpolation - Refactored test code to verify interpolation works with both Julia functions and
DistributedCellFieldobjects
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/FESpaces.jl | Added new overloads for TrialFESpace and interpolate_dirichlet! to support DistributedCellField arguments |
| test/FESpacesTests.jl | Refactored interpolation tests into a helper function and added tests for DistributedCellField interpolation |
| NEWS.md | Documented the new feature in the changelog |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function FESpaces.TrialFESpace(cf::DistributedCellField,f::DistributedSingleFieldFESpace) | ||
| spaces = map(local_views(f),local_views(cf)) do s, field | ||
| TrialFESpace(s,field) | ||
| end | ||
| DistributedSingleFieldFESpace(spaces,f.gids,f.trian,f.vector_type,f.metadata) | ||
| end | ||
|
|
||
|
|
There was a problem hiding this comment.
[nitpick] The two TrialFESpace overloads at lines 417-422 and 424-429 have identical implementations. The second overload (where DistributedCellField is the first argument) simply calls the same underlying Gridap TrialFESpace(s,field) function. Consider consolidating these into a single implementation or adding a comment explaining why both argument orders are necessary.
| function FESpaces.TrialFESpace(cf::DistributedCellField,f::DistributedSingleFieldFESpace) | |
| spaces = map(local_views(f),local_views(cf)) do s, field | |
| TrialFESpace(s,field) | |
| end | |
| DistributedSingleFieldFESpace(spaces,f.gids,f.trian,f.vector_type,f.metadata) | |
| end |
| uh4 = FEFunction(U,free_values,dirichlet_values) | ||
| eh4 = u - uh4 | ||
|
|
||
| dΩ = Measure(Ω,3) |
There was a problem hiding this comment.
The variable dΩ is redefined inside _interpolation_tests at line 131, shadowing the outer dΩ defined at line 88. While this works, it's confusing and the outer dΩ at line 88 appears to be unused. Consider removing the outer definition or using a different variable name in the inner function.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…create_trial_fe_space_out_of_distributed_cell_field
This PR was already merged and released in 0.19.6. Thus this PR is ready to be merged once we confirm that all tests pass. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #185 +/- ##
======================================
Coverage 0.00% 0.00%
======================================
Files 15 15
Lines 3961 3973 +12
======================================
- Misses 3961 3973 +12
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
JordiManyer
deleted the
create_trial_fe_space_out_of_distributed_cell_field
branch
Do not merge yet.
Requires Gridap's PR 1117 to be released.
Related to issue #184